versionsProvidingConfiguration change and ExportAllExcludes#207
versionsProvidingConfiguration change and ExportAllExcludes#207timhamoni wants to merge 3 commits intogradlex-org:mainfrom
Conversation
…ther than a string identifying the configuration. Added the ability to export all packages with the exception of named ones.
jjohannes
left a comment
jjohannes
left a comment
There was a problem hiding this comment.
Thank your for the contribution @timhamoni!
I think the exportAllPackagesExcept is a great addition to this plugin.
I would rather not touch the versionsProvidingConfiguration thing as suggested here (see comment below). I am happy to discuss alternative improvements in this area as a separate issue.
| */ | ||
| private static String pathToPackage(String path) { | ||
| return path.replace('/', '.'); | ||
| } |
There was a problem hiding this comment.
✅ Thanks for introducing these methods!
| // or simply export all packages | ||
| // exportAllPackages() | ||
| // or export all packages except specific named ones | ||
| // exportAllPackagesExcept("org.mycompany.notgood1", "org.mycompany.notgood2") |
| file("src/main/java/module-info.java") << """ | ||
| module org.gradle.sample.app { | ||
| exports org.gradle.sample.app; | ||
|
|
||
| requires org.glassfish.java.json; | ||
| requires java.json; | ||
| } | ||
| """ | ||
| buildFile << """ | ||
| dependencies { | ||
| implementation("org.glassfish:jakarta.json:1.1.6") | ||
| implementation("jakarta.json:jakarta.json-api:1.1.6") | ||
| } | ||
|
|
||
| extraJavaModuleInfo { | ||
| module("org.glassfish:jakarta.json", "org.glassfish.java.json") { | ||
| exportAllPackagesExcept("javax.json", "javax.json.spi", "javax.json.stream") | ||
| overrideModuleName() | ||
| } | ||
| knownModule("jakarta.json:jakarta.json-api", "java.json") | ||
| } | ||
| """ | ||
|
|
||
| expect: | ||
| def result = failRun() | ||
| result.output.matches(/(?s).*Package javax\.json[.a-z]* in both.*/) |
There was a problem hiding this comment.
Thanks for adding a test. Right now it does not seem to test the new feature, though. The split-package error will always occur no matter if the packages are exported or not.
I suggest the following adjustment
| file("src/main/java/module-info.java") << """ | |
| module org.gradle.sample.app { | |
| exports org.gradle.sample.app; | |
| requires org.glassfish.java.json; | |
| requires java.json; | |
| } | |
| """ | |
| buildFile << """ | |
| dependencies { | |
| implementation("org.glassfish:jakarta.json:1.1.6") | |
| implementation("jakarta.json:jakarta.json-api:1.1.6") | |
| } | |
| extraJavaModuleInfo { | |
| module("org.glassfish:jakarta.json", "org.glassfish.java.json") { | |
| exportAllPackagesExcept("javax.json", "javax.json.spi", "javax.json.stream") | |
| overrideModuleName() | |
| } | |
| knownModule("jakarta.json:jakarta.json-api", "java.json") | |
| } | |
| """ | |
| expect: | |
| def result = failRun() | |
| result.output.matches(/(?s).*Package javax\.json[.a-z]* in both.*/) | |
| file("src/main/java/module-info.java") << """ | |
| module org.gradle.sample.app { | |
| exports org.gradle.sample.app; | |
| requires java.json; | |
| } | |
| """ | |
| buildFile << """ | |
| dependencies { | |
| implementation("org.glassfish:jakarta.json:1.1.6") | |
| implementation("jakarta.json:jakarta.json-api:1.1.6") | |
| } | |
| extraJavaModuleInfo { | |
| module("org.glassfish:jakarta.json", "java.json") { | |
| exportAllPackagesExcept("javax.json", "javax.json.spi", "javax.json.stream") | |
| } | |
| } | |
| """ | |
| expect: | |
| def result = failRun() | |
| result.output.contains("error: package javax.json is not visible") |
| public abstract Property<Boolean> getDeriveAutomaticModuleNamesFromFileNames(); | ||
|
|
||
| public abstract Property<String> getVersionsProvidingConfiguration(); | ||
| public abstract Property<Configuration> getVersionsProvidingConfiguration(); |
There was a problem hiding this comment.
I don't think we should change this. If I understand your reasoning correctly, this actually makes the situation worse. The code at the moment always takes the Configuration out of the current project scope – versionsSource = project.getConfigurations()... in PublishedMetadata.java. This ensures that the user cannot put in a Configuration object obtain from a different context.
To solve the warning in you project, you should never do something like this:
configurations.add(project("platform").configurations.named("allDependencies"))
Note: if you turn on org.gradle.unsafe.isolated-projects=true it will find such things in your setup and give you warnings/errors for it.
Instead, you should make the allDependencies a consumable (not resolvable) in platform.
Then in all other projects, you define a dependency to that. Similar to what is documented here. Something like:
// main/platform/build.gradle.kts
val consistentResolutionAttribute = Attribute.of("consistent-resolution", String::class.java)
configurations.create("allDependencies") {
isCanBeResolved = false
...
attributes { attribute(consistentResolutionAttribute, "global") }
}// main/project/build.gradle.kts
val consistentResolutionAttribute = Attribute.of("consistent-resolution", String::class.java)
val allDependenciesPath = configurations.create("allDependenciesPath")) {
isCanBeConsumed = false
attributes { attribute(consistentResolutionAttribute, "global") }
}
dependencies {
allDependenciesPath(project(":platform"))
}
extraJavaModuleInfo {
versionsProvidingConfiguration = "allDependenciesPath"
}This is a bit tricky, but I don't have a more compact solution. And because it is quite individual in different project setups, and cross-cutting subprojects, I don't have a good idea for features in this plugin to make it easier atm.
|
@jjohannes thanks for taking a look at the PRs. Let me do a couple of things, firstly, I'll split the exportAllExcept feature into it's own PR. Then I'll open a discussion item about the configuration related features. I'm just putting together a sample project to demonstrate the two issues I was having and we can see if there's a way forwards. |
jjohannes
added
a:enhancement
|
I moved the I am closing this one as, per my earlier comments, I don't think we can make that change. Please open a new PR or issue if you want to propose/discuss something else in that area. |
|
Hi, thanks. Sorry, I got super busy with some other work. I did create a sample project to try and demonstrate my issue, which then didn't work as I expected. I was trying to figure our why the sample project worked, but my real project did not but never got to diagnosing the root cause. |
jjohannes
removed
a:enhancement
2 participants
Hi,
I've submitted two changes in this. The first is probably the most major.
versionsProvidingConfiguration change
I have changed the versionsProvidingConfiguration from a String to a Configuration. I realise this is a breaking change, but I think it is needed. Gradle 9 introduces extra restrictions around Configurations and their use. A configuration from another project cannot be imported into the current project. The reason I was doing this is as follows.
I have a multi-project build, which do not share a common parent (instead they have a platform project). To obtain a consistent Configuration for resolution, I was adding all dependencies onto a configuration in a holder project, then adding that to each project that used so that the string lookup would work.
For example,
This caused Gradle to issue a warning about the feature being depreciated, and caused the dependencies task to fail.
The updated approach should allow more flexibility, but I realise it will break existing usage.
exportAllPackages enhancement
The second change is to add an exception to exportAllPackages(). I found that a number of packages are "mostly good", and just needed one package not-exporting. In the current version I had to specify all packages to export, using this feature, I can specify which packages not to export.
extraJavaModuleInfo { module("org.example:package", "org.example.package") { exportAllPackagesExcept("org.example.package.hidden") } }